Skip to content

Conversation

@Pirulax
Copy link
Contributor

@Pirulax Pirulax commented May 31, 2020

Basically this PR makes the above mentioned classes use constexpr, and noexcept everywhere they can.

All functions other than the constructors were probably inlined before anyways, but declaring the constructors as constexpr allows me(or someone else), to declare constexpr variables with vectors.
Its kinda useful, just gives me that constifying feel.

These edits are cherry-picked from #1467

@Pirulax
Copy link
Contributor Author

Pirulax commented May 31, 2020

OMG IT WORKS ON THE FIRST TRY jeez m8 thats gr8

Copy link
Contributor

@qaisjp qaisjp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you! I've left a couple style comments, a comment comment, and a suggestion to keep one potential optimisation.

@qaisjp qaisjp added this to the 1.5.8 milestone Jun 3, 2020
@Pirulax
Copy link
Contributor Author

Pirulax commented Jun 6, 2020

I feel like that "potential optimization" is quite unnecessary, as, by default MSVC uses SSE2, which will probably dump all the 4 floats into a m128 reg, and divide them. Which wont be much slower than just doing this optimization trick(which we shoudnt, as the compiler will optimize it, especially if we enable fast-math(which will pretty much guarantee us that the compiler will use SSE2 with this trick we've used:

#pragma fast-math push
#pragma fast-math on
// code
#pragma fast-math pop

@qaisjp qaisjp modified the milestones: 1.5.8, 1.6 Jun 8, 2020
@Pirulax Pirulax mentioned this pull request Jun 11, 2020
@ghost
Copy link

ghost commented Jun 11, 2020

@Pirulax

tbh I agree, but nobody reads CVector code anyways. Constexpr is needed(at least for the constructor) in CGraphics refactor(somewhere in the end, the sphere thing)

Why is constexpr needed there? The main purpose of constexpr is to compute the result in compile time. That can only happen if the compiler knows about the values used in the method at compile-time, otherwise, it will just inline it at best and won't do any good optimizations.

@Pirulax
Copy link
Contributor Author

Pirulax commented Jun 28, 2020

As stated somewhere, I need this for the CGraphics refactor thingy

@ghost ghost merged commit 2a0d3b9 into multitheftauto:master Oct 20, 2020
@botder botder modified the milestones: 1.6, Spring Maintenance Feb 3, 2021
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants